Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DomCrawler] Allowed internal validation of ChoiceFormField to be disabled #8637

Closed

Conversation

pylebecq
Copy link
Contributor

@pylebecq pylebecq commented Aug 1, 2013

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #7672
License MIT
Doc PR Not yet

Hi,

Here is a quite basic attempt to be able to disable the internal validation of the ChoiceFormField. It's pretty basic.
Feel free to tell me what you think guys. Maybe I should check the validationDisabled property at the beginning of the containsOption() method ?
I'll make the documentation PR as soon as the implementation will be validated.

Regards.

@fabpot
Copy link
Member

fabpot commented Aug 2, 2013

What about setting the validation flag on the form itself instead? That way, it would apply on all form fields.

@pylebecq
Copy link
Contributor Author

pylebecq commented Aug 2, 2013

I don't know what is the best.

Choice Pros Cons
Field Can be controlled on a per field basis Tedious to disable validation on a whole form
Form Can disable validation of all fields with 1 method call Cannot disable validation for only one field
Both Can disable validation of all fields with 1 method call or on a per field basis None

For the last option, my proposed implementation is to keep validation flag on the fields and having a method on the Form class to toggle the flag on all the fields.

@pylebecq
Copy link
Contributor Author

pylebecq commented Aug 2, 2013

I made an attempt to implement the "Both" solution of my previous post.

@fabpot What do you think ? Does it fits what you had in mind ? If so, do you think we should introduce an Interface for form fields having validation capabilities, and for the Form class itself ? I did not because only the ChoiceFormField has such capabilities yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants